Skip to content

Conversation

@davidtwco
Copy link
Member

@davidtwco davidtwco commented Nov 24, 2025

Fixes #138261

Adds a namespace for global_asm with a lowercase letter which is reserved for implementation-internal disambiguation:

Lowercase letters are reserved for implementation-internal disambiguation categories (and demanglers should never show them)

As a implementation-internal disambiguation category, the demangler implementations shouldn't need updated (i.e. if this were an uppercase letter, then our mangle-then-demangle checks would fail because the demangler would expect to have explicit handling). 'a' is chosen arbitrarily, for asm, but I can change it to something else if preferred.

#[rustc_symbol_name] only looks at top-level items, and would need a bunch of changes to be able to check the symbol for foo::{constant}::{closure} in the global_asm in this test, so for now the test just checks this compiles.

The alternative to this would be to prohibit declaration of items in the operand of a global_asm, which is a breaking change.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 24, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 24, 2025

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Copy link
Member

@Kivooeo Kivooeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable, I also thinks that 'a' is fine

r=me when ci is green

View changes since this review

@jieyouxu
Copy link
Member

r? Kivooeo

@rustbot rustbot assigned Kivooeo and unassigned jieyouxu Nov 25, 2025
@Kivooeo
Copy link
Member

Kivooeo commented Nov 25, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Nov 25, 2025

📌 Commit 38927ef has been approved by Kivooeo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 25, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 25, 2025
…amespace, r=Kivooeo

add implementation-internal namespace for globalasm

Fixes rust-lang#138261

Adds a namespace for `global_asm` with a lowercase letter which [is reserved for implementation-internal disambiguation](https://doc.rust-lang.org/rustc/symbol-mangling/v0.html#namespace:~:text=Lowercase%20letters%20are%20reserved%20for%20implementation%2Dinternal%20disambiguation%20categories%20(and%20demanglers%20should%20never%20show%20them)):

> Lowercase letters are reserved for implementation-internal disambiguation categories (and demanglers should never show them)

As a implementation-internal disambiguation category, the demangler implementations shouldn't need updated (i.e. if this were an uppercase letter, then our mangle-then-demangle checks would fail because the demangler would expect to have explicit handling). `'a'` is chosen arbitrarily, for **a**sm, but I can change it to something else if preferred.

`#[rustc_symbol_name]` only looks at top-level items, and would need a bunch of changes to be able to check the symbol for `foo::{constant}::{closure}` in the `global_asm` in this test, so for now the test just checks this compiles.

The alternative to this would be to prohibit declaration of items in the operand of a `global_asm`, which is a breaking change.
bors added a commit that referenced this pull request Nov 25, 2025
Rollup of 3 pull requests

Successful merges:

 - #148652 (Cleanup and refactor FnCtxt::report_no_match_method_error)
 - #149268 (add implementation-internal namespace for globalasm)
 - #149274 (Fix invalid link generation for type alias methods)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@bors r-

#149303 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 25, 2025
@davidtwco davidtwco force-pushed the v0-mangling-global-asm-namespace branch from 38927ef to 8ed8f18 Compare November 25, 2025 13:56
@davidtwco
Copy link
Member Author

Oops, was missing the needs-asm-support header in the test, added now.

@Kivooeo
Copy link
Member

Kivooeo commented Nov 25, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 25, 2025

📌 Commit 8ed8f18 has been approved by Kivooeo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 25, 2025
bors added a commit that referenced this pull request Nov 25, 2025
Rollup of 8 pull requests

Successful merges:

 - #147736 (Stabilize `asm_cfg`)
 - #148652 (Cleanup and refactor FnCtxt::report_no_match_method_error)
 - #149167 (skip checking supertraits in object candidate for `NormalizesTo` goal)
 - #149210 (fix: Do not ICE on normalization failure of an extern static item)
 - #149268 (add implementation-internal namespace for globalasm)
 - #149274 (Fix invalid link generation for type alias methods)
 - #149302 (Fix comment wording in simplify_comparison_integral.rs)
 - #149305 (Simplify OnceCell Clone impl)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 50237b3 into rust-lang:main Nov 25, 2025
11 checks passed
@rustbot rustbot added this to the 1.93.0 milestone Nov 25, 2025
rust-timer added a commit that referenced this pull request Nov 25, 2025
Rollup merge of #149268 - davidtwco:v0-mangling-global-asm-namespace, r=Kivooeo

add implementation-internal namespace for globalasm

Fixes #138261

Adds a namespace for `global_asm` with a lowercase letter which [is reserved for implementation-internal disambiguation](https://doc.rust-lang.org/rustc/symbol-mangling/v0.html#namespace:~:text=Lowercase%20letters%20are%20reserved%20for%20implementation%2Dinternal%20disambiguation%20categories%20(and%20demanglers%20should%20never%20show%20them)):

> Lowercase letters are reserved for implementation-internal disambiguation categories (and demanglers should never show them)

As a implementation-internal disambiguation category, the demangler implementations shouldn't need updated (i.e. if this were an uppercase letter, then our mangle-then-demangle checks would fail because the demangler would expect to have explicit handling). `'a'` is chosen arbitrarily, for **a**sm, but I can change it to something else if preferred.

`#[rustc_symbol_name]` only looks at top-level items, and would need a bunch of changes to be able to check the symbol for `foo::{constant}::{closure}` in the `global_asm` in this test, so for now the test just checks this compiles.

The alternative to this would be to prohibit declaration of items in the operand of a `global_asm`, which is a breaking change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ICE: coverage: symbol_names: unexpected DefPathData: GlobalAsm

6 participants